Repository navigation
Conversation
The executor fixes pg_mimic wants next are merged upstream and unreleased: v30.18.0 is the newest release and main is 43 commits past it, carrying 8321 (plan DISTINCT below the Sort so the ORDER BY survives), 8319 and 8304. PyPI rejects a direct-URL dependency, so the commit cannot live in `dependencies`. It goes in tools/install_sqlglot.sh instead, which CI runs after the editable install so it wins over the floor that install pulled in. The floor stays `sqlglot>=30.18.0` -- what a `pip install pg-mimic` actually gets -- and stays exercised daily by the `latest-release` leg of upstream-sqlglot.yml. That leg is also why the one tripwire this fixes is marked conditionally rather than unmarked. 8321 makes `test_select_distinct_ignores_the_order_by_direction` XPASS on the pin, and strict XPASS is a build failure -- but the bug is still live on the floor, so unmarking it would turn the `latest-release` leg permanently red, which is #141's complaint with the legs swapped. Conditional on the version, it passes on the pin and still xfails on the floor. Verified both ways: 964 passed / 19 xfailed on the pin, 963 / 20 on 30.18.0. The workaround it names stays for now. `_sorted_rows` is dead on the pin but not on the floor, where deleting it would answer `SELECT DISTINCT ... ORDER BY DESC` in the wrong order for anyone installing from PyPI. Queued for the floor bump, with a caveat recorded on #49: only the sorting is dead. `_take_result_order` also rejects `SELECT DISTINCT a FROM t ORDER BY b`, which Postgres rejects and sqlglot, SQLite and DuckDB all answer -- arbitrarily, the result flipping with physical row order in all three. That check is Postgres fidelity rather than a workaround, and has to survive the deletion. upstream-sqlglot.yml's main leg now calls the same script, passing `main` for the branch tip, so the clone's two non-obvious details (blobless not shallow, so setuptools_scm can still see tags; hand-cloned because `uv pip install git+...` runs `git submodule update --init` and sqlglot pins a submodule by SSH URL to a private repo) live in one place.
Three of the seven workarounds pg_mimic carries had no strict-xfail tripwire, so nothing would have announced the day upstream made them deletable -- which is the precise failure this file exists to prevent, and which it already records having happened twice (#50). - `_make_decimal_comparisons_exact`: a bare `9.99` is a Python float, and `Decimal("9.99") == 9.99` is False, so `where total = 9.99` misses the row. Separate from the `_cast` tripwire above it, which needs an explicit CAST -- independently fixable, so one tripwire each. - `_distinct_on_keys` / `_first_row_per_key`: the executor parses `DISTINCT ON` and returns the duplicates anyway. The most user-visible of the three, since `DISTINCT ON` is Postgres-specific syntax and a client sending it has already committed to Postgres. - the `REGEXPLIKE` patch in catalog_rewrite.py, the library's only `ENV` patch, on the path of psql's `\d` rather than of anything a user wrote. This is the one that most needed covering: it is an `ENV.setdefault`, so the day upstream adds the function ours goes dead *silently* -- no error, no wrong answer, just code that never runs again. All three expectations verified against PostgreSQL 18.6 rather than asserted from memory, per this file's premise. All three xfail strictly on both the pinned commit and the 30.18.0 floor; 22 xfailed on the pin, 23 on the floor. The module docstring now states the invariant, so the next workaround added without one is a review comment rather than a discovery years later.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The executor fixes pg_mimic wants next are merged upstream and unreleased. v30.18.0 is still the newest release and
mainis 43 commits past it, carrying 8321 (plan DISTINCT below the Sort so theORDER BYsurvives), 8319 and 8304. This develops against that commit.Where the pin lives, and why not in
dependenciesPyPI rejects a direct-URL dependency, so the commit cannot go in
dependencies. It lives intools/install_sqlglot.sh, which CI runs after the editable install so it wins over the floor that install pulled in.The floor stays
sqlglot>=30.18.0— what apip install pg-mimicactually gets — and stays exercised daily by thelatest-releaseleg ofupstream-sqlglot.yml, so flippingtests.ymlto the pin costs no floor coverage.upstream-sqlglot.yml'smainleg now calls the same script withmainfor the branch tip, so the clone's two non-obvious details live in one place: blobless rather than shallow, sosetuptools_scmcan still see tags and derive a version that satisfies our own floor; and hand-cloned, becauseuv pip install git+...runsgit submodule update --initand sqlglot pins a submodule by SSH URL to a private repo no runner can authenticate to.The one tripwire this fixes is marked conditionally, not unmarked
8321 makes
test_select_distinct_ignores_the_order_by_directionXPASS on the pin, and a strict XPASS is a build failure. But the bug is still live on the floor, so unmarking it outright would turn thelatest-releaseleg permanently red — #141's complaint with the legs swapped. Conditional on the version, it passes on the pin and still strict-xfails on the floor.Verified both ways:
3ca8248(30.18.1.dev43)30.18.0As a side effect this should take the
mainleg ofupstream-sqlglot.ymlgreen, since the tree now matches whatmainanswers — which is the half of #141 that made the job's run history unreadable.What this does not delete
Exactly one workaround becomes dead, and only on the pin:
_sorted_rowsand the sort keys intables.py. It stays, because on the floor deleting it would answerSELECT DISTINCT ... ORDER BY ... DESCin the wrong order for anyone installing from PyPI. Queued for the floor bump.With a caveat recorded on #49 for whoever does that bump: only the sorting is dead.
_take_result_orderalso rejectsSELECT DISTINCT a FROM t ORDER BY b, which Postgres rejects (42P10) and sqlglot, SQLite 3.53.4 and DuckDB 1.5.5 all answer — arbitrarily, the result flipping with physical row order in all three, because afterDISTINCT acollapses two rows with the sameaand differentbthere is no singlebleft to sort by. Postgres is the outlier in enforcing it, but that check is Postgres fidelity rather than a workaround, and has to survive the deletion. It has no test today.The other 19 tripwires still xfail on
main: INTERVAL, the exact-decimal cast,TABLESAMPLE,VALUES, rounding, duplicate column names,LIKEescapes,ltrim/rtrim,EXTRACT(dow)and the rest.Issues
No user-visible behaviour changes, so no CHANGELOG entry. Updated out of band:
#123 narrowed to its
_castitem (its_rewrite_not_initems are fixed, by thatworkaround's deletion in b53be1a), and #49 has the new status for the
SELECT DISTINCTrow plus the caveat above.Second commit: every workaround now has a tripwire
Counting what is actually left turned up three workarounds with no strict-xfail tripwire watching them, so nothing would have announced the day upstream made them deletable — the precise failure this file exists to prevent, and which it already records having happened twice (#50). Added one each for
_make_decimal_comparisons_exact,_distinct_on_keys/_first_row_per_key, and theREGEXPLIKEpatch incatalog_rewrite.py.REGEXPLIKEis the one that most needed covering: it is anENV.setdefault, so the day upstream adds the function ours goes dead silently — no error, no wrong answer, just code that never runs again. Nothing else in the suite would have noticed.All three expectations were verified against PostgreSQL 18.6 rather than asserted from memory, per the file's own premise. All three xfail strictly on both the pin and the floor: 22 xfailed on the pin, 23 on the floor. The module docstring now states the invariant, so the next workaround added without one is a review comment rather than a discovery years later.
For the record, the workaround count this leaves: six live in the library (
_cast,_make_decimal_comparisons_exact,DISTINCT ON,canonicalize_table_aliases,_flatten_parenthesized,REGEXPLIKE), one required only on the floor (_sorted_rows), and two inexamples/git_sql.py(the clock, andINTERVAL). The 19 tripwires still red are mostly not workarounds — 13 of them are untreated gaps that reach users (#58).